-
Notifications
You must be signed in to change notification settings - Fork 1
Handle composed and mixed associated type constraints #108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Handle composed and mixed associated type constraints #108
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #108 +/- ##
==========================================
+ Coverage 68.78% 68.84% +0.06%
==========================================
Files 71 71
Lines 4558 4567 +9
==========================================
+ Hits 3135 3144 +9
Misses 1423 1423 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small formatting suggestion and one refactoring request:
for (index, inheritedType) in inheritedTypes.enumerated() { | ||
inheritedType | ||
.trimmed | ||
.with(\.ampersand, index < lastIndex ? .binaryOperator("&") : nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.binaryOperator("&")
is the ampersand token used in Swift Syntax's CompositionTypeElementListSyntax
by default.
@graycampbell I’ve updated the code to build the inheritance clause using a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
📝 Summary
This pull request fixes a bug in
MockedMacro.mockGenericParameterClause
where associated types using composition (&
) in their constraints weren’t handled correctly.Previously, the
@Mocked
assumed all inherited types wereIdentifierTypeSyntax
nodes, which holds for comma-separated constraints (e.g.Hashable, Sendable
). However, composed constraints (e.g.Hashable & Sendable
) are represented in the syntax tree asCompositionTypeSyntax
, which nests multipleIdentifierTypeSyntax
elements. As a result, these weren’t being parsed, and the generated generic parameter clause was incomplete or incorrect.To fix this, I've:
mockGenericParameterClause
to correctly extract constraints from bothIdentifierTypeSyntax
and nestedCompositionTypeSyntax
.🛠️ Type of Change
🧪 How Has This Been Tested?
I've tested the
@Mocked
macro expansion when anassociatedtype
has constraints that are comma-separated, composed, or a mix of both. Please see screenshots of the expansion below.I've also added unit tests to validate these scenarios going forward.
@Mocked
expansion🔗 Related PRs or Issues
✅ Checklist
SwiftFormat